Skip to content

feat(web): redesign files, sources, and staged deletions - #1588

Open
rodboev wants to merge 63 commits into
pr/redesign-4-messagesfrom
pr/redesign-5-files
Open

rodboev wants to merge 63 commits into
pr/redesign-4-messagesfrom
pr/redesign-5-files

Conversation

@rodboev

@rodboev rodboev commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Files now uses the Messages toolbar, one Filter menu, and a viewer with Add to task. Sources keeps mail and Contacts sync independent, shows schedules and runs, and opens repair Settings on the selected account. This slice extends the toolbar from #1579. Before, Files split its filters, Contacts lived in Settings, and staged deletions showed manifest IDs.

Staged deletions lists each batch by date, message count, and account. The Messages selection bar opens a review dialog before staging; msgvault delete-staged runs deletion. The sidebar shows Staged deletions while a batch is pending, running, or failed.

The deletion list API now returns each batch's account. Screenshots use synthetic data in dark mode at 1280×720. The People list and page build on this in #1611.

Before After
Files Files: separate filename and Type controls Files: one Filter menu and Set up action
Sources Sources: schedule and sync columns Sources: Contacts group and account panel
Contacts Contacts in Settings Contacts in Sources with sync recovery
Contacts repair Contacts repair before: Settings opens the default account Contacts repair after: Settings opens the selected account
Staged deletions Deletions with manifest IDs Staged deletions with date, count, and account

Refs #1552, slice 5 (Files, Sources, and Staged deletions)
Stacks on #1579

@rodboev
rodboev added this pull request to stack #1561 October 30, 2026 07:11
@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (ce8bebc)

Verdict: Changes require fixes for 3 findings.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:241: If the initial mail-status request fails while contacts report an active run, polling stops. Contacts remain stuck showing Running, with Sync now unavailable after completion until manual refresh or remount.

    Fix: Continue polling active contacts independently of mail-status loading success. Cover a contacts run completing after the initial mail-status failure.

  • web/src/lib/components/sources/SourcesWorkspace.svelte:162: In a contacts-only archive, every background poll sets loading=true because sources is empty. This replaces populated rows with Loading, removes focused controls, and disrupts keyboard navigation.

    Fix: Show initial loading only when both sources and contacts are empty; retain existing rows during refresh.

  • web/src/lib/sources/contacts.ts:34: Partial contacts runs can contain an error_message when some address books succeed and another fails, but the failed-only condition discards that diagnostic. The panel shows Partial and counters without explaining the failure.

    Fix: Display the latest run's error_message for partial as well as failed results, preserving repair-reason precedence. Cover a partially successful contacts sync.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 15s | Total: 24m40s

@rodboev
rodboev force-pushed the pr/redesign-5-files branch from ce8bebc to 5d00bcb Compare October 30, 2026 09:31
@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (5d00bcb)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/deletions/controller.svelte.ts:65: Refreshing an open in-progress batch on browser focus deletes its cached details even when its status is unchanged, unmounting the panel. If the replacement request fails or times out, the panel stays closed without explanation, hiding progress and the Cancel batch action.

    Fix: Keep existing details visible during refresh, replace them only after a successful response, and surface refresh failures. Add a regression test for this failure path.

  • web/src/lib/deletions/controller.svelte.ts:84: Closing and reopening a batch returns cached execution details even through getDetail(id, true). If a CLI deletion advances or finishes while the browser remains focused, progress and cancellation availability can remain stale indefinitely because no polling refreshes the cache.

    Fix: Fetch fresh details when explicitly reopening a mutable batch, or periodically refresh active batches and reconcile their summary status.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 13s | Total: 16m24s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (9b6183a)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/components/deletions/DeletionsWorkspace.svelte:68: Successful focus refreshes fetch final deletion details, but the open panel retains its original snapshot, leaving counts and failed message IDs stale while status updates.

    Fix: Update the open panel after successful detail refreshes, retaining previous details while refreshes are pending or fail. Cover successful focus refresh in a regression test.

  • web/src/lib/deletions/controller.svelte.ts:105; web/src/lib/components/deletions/DeletionsWorkspace.svelte:68: Reopening a completed deletion batch fetches fresh details but derives status and cancellation availability from the stale list summary, so the panel can continue showing Deleting and offering Cancel batch.

    Fix: Reconcile successful detail responses into the list summary so fresh status controls the status display and cancellation availability. Cover reopening a completed batch in a regression test.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 17s | Total: 15m50s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (01c3ed4)

Verdict: Changes require fixes for 1 finding.

Medium

  • web/src/lib/components/deletions/DeletionsWorkspace.svelte:69: A status-changing focus refresh can supersede the detail request awaited by open(), which returns undefined. The workspace treats this as a failure and clears selectedID, closing the panel even when the replacement request succeeds.

    Fix: Distinguish superseded requests from failures and await the replacement detail request without clearing the selected batch. Cover a status-changing refresh during an outstanding open request.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 13s | Total: 14m26s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (74f7253)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 9s | Total: 13m55s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (313004a)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 8s | Total: 13m13s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (d4efb4b)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 10s | Total: 14m59s

@rodboev
rodboev force-pushed the pr/redesign-5-files branch from d4efb4b to be1889b Compare October 30, 2026 18:14
@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (be1889b)

Verdict: Changes require fixes for 1 finding.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:558: Table wrappers using flex: none cannot shrink when rows exceed the viewport, clipping lower source rows. The same issue at web/src/lib/components/deletions/DeletionsWorkspace.svelte:236 hides older deletion batches and the footer because neither workspace nor its height-constrained parent scrolls.

    Fix: Allow both table wrappers to shrink with min-height: 0, or make their workspace containers scroll vertically.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 16s | Total: 13m49s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (f9400d9)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 10s | Total: 15m16s

@rodboev
rodboev force-pushed the pr/redesign-5-files branch from f9400d9 to 41e245c Compare October 30, 2026 19:22
@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (41e245c)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/components/sources/ContactPanel.svelte:54: Active source panels hide sync request failures and status retry controls. ContactPanel omits controller.syncError and statusError, while SourcePanel never receives triggerError. On narrow screens, the panel obscures these errors on the underlying page.

    Fix: Render controller.syncError and statusError with appropriate retry controls in ContactPanel, and pass triggerError into SourcePanel for display.

  • web/src/lib/components/deletions/DeletionReview.test.ts:39: The zero-eligible-messages test checks that staging is disabled during initial loading, before awaiting the preflight reason. It can pass even if the completed review incorrectly enables staging.

    Fix: Await 'No deletable messages.' before asserting that the 'Stage 0 messages' button is disabled. Retain the reason-display assertion.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 12s | Total: 14m45s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (0204971)

Verdict: Changes require fixes for 1 finding.

Medium

  • web/src/lib/components/tasks/TaskLinks.test.ts:149: The Tasks fixture returns a successful integration status of 'unavailable', which production evaluators do not produce and TaskIntegrationStatusResponse excludes. MessageCard.test.ts:258 repeats this unsupported state. The Tasks fixture also fails to exercise recovery of cached links after a status-request failure.

    Fix: Make the Tasks status request return HTTP 503 while retaining the cached lookup response. Replace the Kata 'unavailable' case with a supported terminal state such as 'disabled'.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 13s | Total: 19m43s

@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (2317135)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 12s | Total: 17m43s

@rodboev
rodboev force-pushed the pr/redesign-5-files branch from 2317135 to 9a1b35d Compare October 30, 2026 20:06
@roborev-ci

roborev-ci Bot commented Oct 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (9a1b35d)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 7s | Total: 13m7s

@rodboev
rodboev marked this pull request as ready for review October 30, 2026 20:23
@rodboev
rodboev removed this pull request from stack #1561 October 31, 2026 16:18
@rodboev
rodboev added this pull request to stack #1624 October 31, 2026 16:18
@rodboev
rodboev marked this pull request as draft October 31, 2026 17:26
rodboev and others added 8 commits November 2, 2026 01:04
Directory kept its Filters button while Messages and Files moved to
Filter. The shared command needs to reach each workspace's button so
keyboard and command palette users can open Directory filters again.
An older list response could restore a cancelled batch to staged or deleting.
If the next refresh failed, the table and sidebar kept that stale status.
Keep the successful cancellation authoritative while that list finishes.
The Files changes carried older Settings code over the shared status and People
profile refreshes. Restore the parent layer's reload behavior so saved profiles,
Contacts status, and failed index refreshes remain accurate. People writes keep
the revision returned with the profile they display.

Generated with Codex (gpt-6.1-sol)
Returning to Sources can leave an older connections snapshot in flight
while Sync now finishes. That snapshot restored stale failure details and
last-good timestamps after the idle controller had stopped polling.

Cancel the competing list refresh through its existing request controller
so the completed sync remains visible in both the row and account panel.
A connections refresh can start while contacts sync is still waiting and
return an old failure after sync completes. Terminal syncs stop polling,
so that response can leave the row and account panel stale indefinitely.

Cancel the competing request when sync finishes so refreshes started
during the sync also preserve its reconciled result.
…redesign-5-files

# Conflicts:
#	web/src/lib/components/settings/SettingsWorkspace.svelte
#	web/src/lib/components/settings/SettingsWorkspace.test.ts
Renaming a saved view or saving its description could leave the editor
busy after the save succeeded. A slow staged-deletion request held up
the shared refresh even though saved views had already loaded.

Saved-view edits need to wait only for their own refreshed list.
The Files branch lost Past runs coverage while archive navigation behavior
stayed the same. Restore those checks so history regressions remain visible.

Duplicate cases and unrealistic fetch behavior obscured what the remaining
tests protected. Keep each scenario focused on distinct, reachable behavior.
@rodboev
rodboev marked this pull request as ready for review November 2, 2026 01:09
Most source-label rows supply only the type and display name. Mark the
mailbox arguments optional so the shared callback accepts both fixture
shapes during TypeScript validation.

Generated with Codex
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (ba82c51)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 24m19s

Batches from different mailboxes can share a date and message count.
Show their accounts in the list so users can distinguish them without
opening each panel or fetching details for every row.
Repeated list scenarios increased maintenance without protecting another behavior. Realistic API fixtures and independent consent and row-count checks make failures point to reachable regressions.
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (ea50e26)

Verdict: No findings at or above medium severity.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex | Total: 18m45s

@rodboev
rodboev marked this pull request as draft November 2, 2026 15:14
# Conflicts:
#	api/openapi.yaml
#	docs/api-server.md
#	internal/api/carddav.go
#	internal/api/openapi.go
#	pkg/client/openapi.yaml
#	web/src/lib/components/settings/CardDAVOperations.svelte
#	web/src/lib/components/sources/SourcesWorkspace.svelte
#	web/src/lib/components/sources/SourcesWorkspace.test.ts
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (06ad28a)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:147: Initially idle scheduled contacts connections never start controller polling because only active connections call retryStatus(). Mail polling does not refresh contacts, so scheduled runs, failures, and last-good timestamps remain stale until an explicit refresh, visibility change, or remount.

    Fix: Start controller polling for scheduled, queued, or scheduler-running connections, including initially idle scheduled accounts. Add coverage for an idle scheduled connection progressing to completion while Sources remains open.

  • web/src/lib/sources/contacts.ts:8: contactRunState ignores queued_at and scheduler_running. Contacts jobs waiting for the scheduler’s work gate therefore display the previous result or “Never synced” instead of their current queued or running state, even after fresh status responses.

    Fix: Include scheduler_running and queued_at when deriving contacts status while retaining failure diagnostics.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 17s | Total: 23m17s

Scheduled contacts runs can start while Sources stays open. Watching only
active runs left these accounts showing an old result and last good sync
until the user refreshed the page.

The scheduler can queue or start work before an active run exists. Those
states need to stay visible, while a queued retry must preserve the
previous failure so the user can still diagnose it.
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (fdd58dc)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:147: A delayed Connections refresh can resolve during manual sync reconciliation, overwriting newly committed successful status with a previous failure or aborting reconciliation’s status read through retryStatus(), causing syncUnknown and disabling Sync now. The abort in syncContacts() occurs too late to prevent these outcomes.

    Fix: Coordinate connection refreshes with the controller’s sync/status revision so stale summaries cannot overwrite newer status or supersede reconciliation reads. Cover refresh completion during reconciliation.

  • web/src/lib/sources/labels.test.ts:24: The test 'extracts the mailbox from an IMAP source identifier' duplicates the exact input and expected result already exercised through the real sourceMailbox function by 'shows mailbox and source type labels in file rows' in web/src/lib/components/files/FilesWorkspace.test.ts:241. It adds no distinct boundary coverage.

    Fix: Delete the standalone test and its now-unused sourceMailbox import. Retain the component test.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 12s | Total: 20m26s

A delayed Connections response could restore an old failure while a
manual sync was reconciling, or cancel the status read that decides
whether Sync now can be enabled. Summaries need the same ordering
guarantees as the controller's own reads.

The focus regression used the old Deletions label, so it failed before
reaching browser history navigation. The duplicate mailbox example adds
no coverage beyond the existing file-row test.
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (e5deb2b)

Verdict: Changes require fixes for 2 findings.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:532: When a sync returns 202 but no new run appears before maxAwaitingPolls, the open source panel silently returns to its previous result. The not_observed notice and Refresh action remain behind the panel and are invisible on narrow screens.

    Fix: Pass the source’s awaiting/not_observed state into SourcePanel, display the notice and Refresh action there, and cover this outcome when syncing from the panel.

  • web/src/lib/carddav/controller.svelte.ts:106: Accepting an idle Connections summary leaves statusError unchanged. After a failed status request for an unscheduled account, a successful Sources refresh clears syncUnknown but retains the stale error, leaving the row and panel showing failure and Sync now disabled despite receiving current status.

    Fix: Clear statusError when committing an authoritative Connections summary and cover recovery after a failed status read.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 13s | Total: 17m52s

@rodboev
rodboev marked this pull request as ready for review November 2, 2026 17:01
A failed status read kept Sync now disabled even after Sources received
current connection status. Accepting that summary must clear the stale
request error so the account can recover without another status request.
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (229e735)

Verdict: Changes require fixes for 1 finding.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:532: When sync returns 202 but no new run appears before maxAwaitingPolls, the open source panel silently returns to its previous result. The not_observed notice and Refresh action remain behind the panel and are hidden on narrow screens.

    Fix: Pass the source’s awaiting/not_observed state into SourcePanel, display its notice and Refresh action there, and cover this outcome when starting sync from the panel.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 11s | Total: 13m16s

The status response now requires scheduler_running, so the type check rejected the idle fixture once a test passed it straight to refreshFromConnection.
@roborev-ci

roborev-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (2ee5618)

Verdict: Changes require fixes for 1 finding.

Medium

  • web/src/lib/components/sources/SourcesWorkspace.svelte:532: When a sync returns 202 but no new run appears before maxAwaitingPolls, the open source panel reverts to its previous result without explaining that the requested run was not observed. The notice and Refresh action are behind the panel and obscured on narrow screens.

    Fix: Pass source-specific awaiting/not_observed state into SourcePanel, display the notice and Refresh action there, and test reaching the polling cap while the panel is open.


Reviewers: 2x codex, codex (security), codex (test-value) | Synthesis: codex, 10s | Total: 15m27s

@rodboev

rodboev commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Risk accepted: when a sync request is accepted but no new run shows up before polling stops, the "not observed" notice and Refresh button sit behind the open source panel.

Example: you click Sync in the panel, the server says OK, but the run never appears. The panel goes back to showing the last result, and the explanation is on the page underneath it. Closing the panel shows it. This only happens when the server accepts a sync and then never starts it, which is rare. Moving the notice into the panel means threading per-source polling state into it, which belongs with a later panel cleanup rather than this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants